Rewrite pam faillock related implementation and template #12654

alanmcanonical · 2024-11-29T23:30:51Z

Description:

Rewrite pam faillock related implementation and template
Use pam-auth-update to implement dynamic pam stack modification
Clean up duplicated accounts_passwords_pam_faillock_deny tests
Remove general template tests

Rationale:

Use clean pam-auth-update configuration to override the /etc/pam.d/common-* configuration instead of modifying PAM files directly
There is no way to initialize the {{{ EXT_VARIABLE }}} in tests scripts which later will init to var_accounts_passwords_pam_faillock_{deny, unlock_time, interval} in bash remediation, at least no to my knowledge.

openshift-ci · 2024-11-29T23:31:02Z

Hi @alanmcanonical. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

github-actions · 2024-11-29T23:32:33Z

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)

Fedora Testing Environment

Oracle Linux 8 Environment

github-actions · 2024-11-29T23:34:52Z

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff

bash remediation for rule 'xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_audit' differs.
--- xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_audit
+++ xccdf_org.ssgproject.content_rule_account_passwords_pam_faillock_audit
@@ -27,14 +27,16 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*audit"
     line="audit"
     if ! grep -q $regex $FAILLOCK_CONF; then
         echo $line >> $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -82,6 +84,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_audit' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_audit
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_audit
@@ -29,14 +29,16 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*audit"
     line="audit"
     if ! grep -q $regex $FAILLOCK_CONF; then
         echo $line >> $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -84,6 +86,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny
@@ -32,9 +32,10 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*deny\s*="
     line="deny = $var_accounts_passwords_pam_faillock_deny"
     if ! grep -q $regex $FAILLOCK_CONF; then
@@ -42,6 +43,7 @@
     else
         sed -i --follow-symlinks 's|^\s*\(deny\s*=\s*\)\(\S\+\)|\1'"$var_accounts_passwords_pam_faillock_deny"'|g' $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -89,6 +91,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny_root' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny_root
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_deny_root
@@ -29,14 +29,16 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*even_deny_root"
     line="even_deny_root"
     if ! grep -q $regex $FAILLOCK_CONF; then
         echo $line >> $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -84,6 +86,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_dir
@@ -32,9 +32,10 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*dir\s*="
     line="dir = $var_accounts_passwords_pam_faillock_dir"
     if ! grep -q $regex $FAILLOCK_CONF; then
@@ -42,6 +43,7 @@
     else
         sed -i --follow-symlinks 's|^\s*\(dir\s*=\s*\)\(\S\+\)|\1'"$var_accounts_passwords_pam_faillock_dir"'|g' $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -89,6 +91,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_enforce_local' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_enforce_local
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_enforce_local
@@ -29,14 +29,16 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*local_users_only"
     line="local_users_only"
     if ! grep -q $regex $FAILLOCK_CONF; then
         echo $line >> $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -84,6 +86,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_interval' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_interval
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_interval
@@ -32,9 +32,10 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*fail_interval\s*="
     line="fail_interval = $var_accounts_passwords_pam_faillock_fail_interval"
     if ! grep -q $regex $FAILLOCK_CONF; then
@@ -42,6 +43,7 @@
     else
         sed -i --follow-symlinks 's|^\s*\(fail_interval\s*=\s*\)\(\S\+\)|\1'"$var_accounts_passwords_pam_faillock_fail_interval"'|g' $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -89,6 +91,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

bash remediation for rule 'xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_unlock_time' differs.
--- xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_unlock_time
+++ xccdf_org.ssgproject.content_rule_accounts_passwords_pam_faillock_unlock_time
@@ -32,9 +32,10 @@
 fi
 
 AUTH_FILES=("/etc/pam.d/system-auth" "/etc/pam.d/password-auth")
+SKIP_FAILLOCK_CHECK=false
 
 FAILLOCK_CONF="/etc/security/faillock.conf"
-if [ -f $FAILLOCK_CONF ]; then
+if [ -f $FAILLOCK_CONF ] || [ "$SKIP_FAILLOCK_CHECK" = "true" ]; then
     regex="^\s*unlock_time\s*="
     line="unlock_time = $var_accounts_passwords_pam_faillock_unlock_time"
     if ! grep -q $regex $FAILLOCK_CONF; then
@@ -42,6 +43,7 @@
     else
         sed -i --follow-symlinks 's|^\s*\(unlock_time\s*=\s*\)\(\S\+\)|\1'"$var_accounts_passwords_pam_faillock_unlock_time"'|g' $FAILLOCK_CONF
     fi
+    
     for pam_file in "${AUTH_FILES[@]}"
     do
         if [ -e "$pam_file" ] ; then
@@ -89,6 +91,7 @@
             echo "$pam_file was not found" >&2
         fi
     done
+    
 else
     for pam_file in "${AUTH_FILES[@]}"
     do

mpurg

Few minor changes needed

mpurg · 2024-12-01T14:16:48Z

shared/templates/pam_account_password_faillock/tests/ubuntu_missing_pamd.fail.sh


-sed -i '/pam_faillock\.so/d' /etc/pam.d/common-auth
-sed -i '/pam_faillock\.so/d' /etc/pam.d/common-account
+DEBIAN_FRONTEND=noninteractive pam-auth-update

 echo "deny=1" > /etc/security/faillock.conf


The parameter deny should not be hardcoded in the test since it's not used in other rules that use the template:

accounts_passwords_pam_faillock_unlock_time

accounts_passwords_pam_faillock_interval

Use {{{ PRM_NAME }}}, {{{ VARIABLE_UPPER_BOUND }}}, and {{{ VARIABLE_LOWER_BOUND }}} instead.

mpurg · 2024-12-01T14:17:39Z

shared/templates/pam_account_password_faillock/tests/ubuntu_wrong_value.fail.sh

@@ -1,6 +1,5 @@
 #!/bin/bash
 # platform = multi_platform_ubuntu

-source ubuntu_common.sh
-
+{{{ bash_enable_pam_faillock_directly_in_pam_files() }}}
 echo "deny=999" > /etc/security/faillock.conf


Same as previous comment.

.github/workflows/automatus-ubuntu2404.yml

dodys

needs a rebase now that the automatus changes were merged

Add correct os into applicable platform Update the ubuntu oval to pass on both required and requisite Remove the check for authsucc since preauth can clear the count too Implement individual tests for pam_faillock_{deny, unlock_time, interval}

codeclimate · 2024-12-06T15:08:18Z

Code Climate has analyzed commit cf728c8 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 60.9% (0.0% change).

View more on Code Climate.

dodys

lgtm, thanks!

openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Nov 29, 2024

alanmcanonical force-pushed the ubuntu_enable_faillock branch from 90c4587 to 34aaa9c Compare December 1, 2024 14:00

mpurg suggested changes Dec 1, 2024

View reviewed changes

dodys requested changes Dec 2, 2024

View reviewed changes

dodys added the Update Template Issues or pull requests related to Templates updates. label Dec 2, 2024

dodys added this to the 0.1.76 milestone Dec 2, 2024

Ubuntu: enable pam_faillock.so via pam-auth-update

9dee381

alanmcanonical force-pushed the ubuntu_enable_faillock branch from 34aaa9c to 85023f3 Compare December 2, 2024 09:53

alanmcanonical marked this pull request as draft December 2, 2024 11:00

openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Dec 2, 2024

dodys self-assigned this Dec 2, 2024

alanmcanonical force-pushed the ubuntu_enable_faillock branch from 83e684f to ab751cd Compare December 2, 2024 16:01

dodys added the Ubuntu Ubuntu product related. label Dec 2, 2024

alanmcanonical marked this pull request as ready for review December 2, 2024 16:10

openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Dec 2, 2024

alanmcanonical marked this pull request as draft December 2, 2024 18:56

openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Dec 2, 2024

alanmcanonical force-pushed the ubuntu_enable_faillock branch 2 times, most recently from 3485720 to e621940 Compare December 6, 2024 14:18

Add 'required' check in oval

cf728c8

Add correct os into applicable platform Update the ubuntu oval to pass on both required and requisite Remove the check for authsucc since preauth can clear the count too Implement individual tests for pam_faillock_{deny, unlock_time, interval}

alanmcanonical force-pushed the ubuntu_enable_faillock branch from e621940 to cf728c8 Compare December 6, 2024 14:39

alanmcanonical marked this pull request as ready for review December 9, 2024 09:49

openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Dec 9, 2024

dodys approved these changes Dec 10, 2024

View reviewed changes

dodys merged commit f1d163c into ComplianceAsCode:master Dec 10, 2024
93 of 98 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite pam faillock related implementation and template #12654

Rewrite pam faillock related implementation and template #12654

alanmcanonical commented Nov 29, 2024 •

edited

Loading

openshift-ci bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024 •

edited

Loading

mpurg left a comment

mpurg Dec 1, 2024

mpurg Dec 1, 2024

dodys left a comment

codeclimate bot commented Dec 6, 2024

dodys left a comment

Rewrite pam faillock related implementation and template #12654

Rewrite pam faillock related implementation and template #12654

Conversation

alanmcanonical commented Nov 29, 2024 • edited Loading

Description:

Rationale:

openshift-ci bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024 • edited Loading

mpurg left a comment

Choose a reason for hiding this comment

mpurg Dec 1, 2024

Choose a reason for hiding this comment

mpurg Dec 1, 2024

Choose a reason for hiding this comment

dodys left a comment

Choose a reason for hiding this comment

codeclimate bot commented Dec 6, 2024

dodys left a comment

Choose a reason for hiding this comment

alanmcanonical commented Nov 29, 2024 •

edited

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading